-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Initial update to a couple of Management API endpoints #20549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Initial update to a couple of Management API endpoints #20549
Conversation
Making a small change to the Readme to signpost the Forum now that it's the place to go for help/questions
Update README.md with information about the forum
|
Hi there @OwainWilliams, thank you for this contribution! 👍 While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds EndpointSummary and EndpointDescription attributes to selected Management API endpoints to improve Swagger/OpenAPI documentation.
- Adds summaries/descriptions to DocumentVersion endpoints (list, get-by-id, rollback, set prevent-cleanup).
- Adds summaries/descriptions to Culture list endpoint.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/UpdatePreventCleanupDocumentVersionController.cs | Adds summary/description explaining the prevent-cleanup flag operation. |
| src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/RollbackDocumentVersionController.cs | Adds summary/description for rolling back a document to a specific version. |
| src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/ByKeyDocumentVersionController.cs | Adds summary/description for fetching a specific document version by ID. |
| src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/AllDocumentVersionController.cs | Adds summary/description for listing versions of a document (paged). |
| src/Umbraco.Cms.Api.Management/Controllers/Culture/AllCultureController.cs | Adds summary/description for listing available cultures (paged). |
src/Umbraco.Cms.Api.Management/Controllers/Culture/AllCultureController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/Culture/AllCultureController.cs
Outdated
Show resolved
Hide resolved
....Api.Management/Controllers/DocumentVersion/UpdatePreventCleanupDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/AllDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/ByKeyDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
…ontroller.cs Co-authored-by: Copilot <[email protected]>
…atePreventCleanupDocumentVersionController.cs Co-authored-by: Copilot <[email protected]>
…eyDocumentVersionController.cs Co-authored-by: Copilot <[email protected]>
…DocumentVersionController.cs Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much for kicking this off @OwainWilliams. I had a look over and made a few suggestions. Please see what you think - in some cases you may not agree and if so, please do say so or suggest further improvement. We can tweak further perhaps to set the pattens moving forward.
In the suggestions I've tried to standardise a little so we consistently use certain terms and punctuation, have simplified in some cases so we don't create an undue burden on ourselves going forward in describing every field or parameter, and avoided repeating something that's already in the Swagger UI. Finally, in a couple of cases I focussed more on the end result of the operation (so not just saying a flag is set for content clean up, but what this means in terms of the system behaviour).
src/Umbraco.Cms.Api.Management/Controllers/Culture/AllCultureController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/Culture/AllCultureController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/AllDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/AllDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/ByKeyDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/ByKeyDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/RollbackDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Cms.Api.Management/Controllers/DocumentVersion/RollbackDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
....Api.Management/Controllers/DocumentVersion/UpdatePreventCleanupDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
....Api.Management/Controllers/DocumentVersion/UpdatePreventCleanupDocumentVersionController.cs
Outdated
Show resolved
Hide resolved
…ontroller.cs Co-authored-by: Andy Butland <[email protected]>
…ontroller.cs Co-authored-by: Andy Butland <[email protected]>
…DocumentVersionController.cs Co-authored-by: Andy Butland <[email protected]>
…DocumentVersionController.cs Co-authored-by: Andy Butland <[email protected]>
…eyDocumentVersionController.cs Co-authored-by: Andy Butland <[email protected]>
…eyDocumentVersionController.cs Co-authored-by: Andy Butland <[email protected]>
…lbackDocumentVersionController.cs Co-authored-by: Andy Butland <[email protected]>
…lbackDocumentVersionController.cs Co-authored-by: Andy Butland <[email protected]>
…atePreventCleanupDocumentVersionController.cs Co-authored-by: Andy Butland <[email protected]>
…atePreventCleanupDocumentVersionController.cs Co-authored-by: Andy Butland <[email protected]>
|
Thanks @AndyButland - really appreciate the feedback. Sorry you had to do so much tidy up on it :) Hopefully it will give me and others a good guide though on what sort of language and info should be added to the summary and descriptions |
|
You're welcome - looks good to me now and I think is a reasonable reference also for moving forward. @kjac - do you have any thoughts at this stage please, before @OwainWilliams or anyone else looks to create further endpoint descriptions? |
|
I have thoughts. I think it's awesome 🤩 |
|
Good to hear! So @OwainWilliams - please let us know what your plans are for these now. Although little and often is fine, it might make sense to do another round with a few more, and then we look to merge in and take others in future PRs. I think you should also retarget this PR to |
|
I'll see if I can get another couple of smaller sections done over the coming days. Don't want to lose momentum on this. Maybe target a few that are further up the list on the swagger page. Then I'd aim to keep that going, tick off more over the coming weeks. How does that sound? |
…terDataTypeFilterController.cs Co-authored-by: Andy Butland <[email protected]>
…eyDataTypeFolderController.cs Co-authored-by: Andy Butland <[email protected]>
…renDataTypeTreeController.cs Co-authored-by: Andy Butland <[email protected]>
…eyDataTypeFolderController.cs Co-authored-by: Andy Butland <[email protected]>
…eteDataTypeFolderController.cs Co-authored-by: Andy Butland <[email protected]>
…ateDataTypeFolderController.cs Co-authored-by: Andy Butland <[email protected]>
…eteDataTypeFolderController.cs Co-authored-by: Andy Butland <[email protected]>
…renDataTypeTreeController.cs Co-authored-by: Andy Butland <[email protected]>
…ateDataTypeFolderController.cs Co-authored-by: Andy Butland <[email protected]>
…ateDataTypeFolderController.cs Co-authored-by: Andy Butland <[email protected]>
…atatypeItemController.cs Co-authored-by: Andy Butland <[email protected]>
…ies' of https://github.com/OwainWilliams/Umbraco-CMS into v17/task/updating-managment-api-descritpions-and-summaries
…atatypeItemController.cs Co-authored-by: Andy Butland <[email protected]>
…torsDataTypeTreeController.cs Co-authored-by: Andy Butland <[email protected]>
…hDataTypeItemController.cs Co-authored-by: Andy Butland <[email protected]>
…peController.cs Co-authored-by: Andy Butland <[email protected]>
…peController.cs Co-authored-by: Andy Butland <[email protected]>
…/ReferencedByDataTypeController.cs Co-authored-by: Andy Butland <[email protected]>
…torsDataTypeTreeController.cs Co-authored-by: Andy Butland <[email protected]>
…/ReferencedByDataTypeController.cs Co-authored-by: Andy Butland <[email protected]>
…ataTypeTreeController.cs Co-authored-by: Andy Butland <[email protected]>
|
@OwainWilliams - please see what you think: #20690. I gave this a try:
And it seemed to work as I'd hoped after a bit of back and forth as you'll see. Now we had some good examples, we could lean on this to at least to a pretty good first draft of the descriptions. Up to you, but perhaps it'll be a fair bit quicker for you to review and suggest updates to this - either via the GitHub UI, or take a branch from this PR branch and make a PR from your fork back to it - than to write them all from scratch. |
|
@AndyButland Looks like you had a long conversation with copilot 😄 Im a bit confused though on the next steps - did you want me to run copilot over this PR or leave this one as it is and continue working on #20690 or something else? Happy to do what ever is needed, just wasn't 100% sure on what was being suggested. |
|
Of course it's all up to you, but I was thinking we could:
|
|
Ah, great, thanks @AndyButland - That all makes sense. I'm happy to take a look at your PR, make adjustments to it and go from there. I don't see any point forking from your work already - seems like an unrequired extra step. Lets close this one off and I'll take a look at your other PR and make any suggestions |
|
No need to close this one - I would say for the endpoints it covers, it takes precedence over the other, as it established the patterns and I'd rather we have the human ones as preference! But suggest we keep it open until the other one is reviewed, and then we can have all merged in at once. |
|
Sorry - when I said Close, I meant, not do any more to this one and I'll pick up the other PR :) |
After an initial conversation on #20186 I've started working on adding Summary and Descriptions to the Management API Swagger docs.
This is an update to the Culture and DocumentVersion endpoints. Started with a couple of the smaller endpoints so that I can make updates if required and get an idea of what is being looked for.